Looks good to me. You could consider distinguishing between one or more missing arguments (argument vs arguments). I think most other error messages do this.

@spkersten, sounds good to me, I somewhat expected that the format of the message would be discussed in the pull request. :) @JukkaL, how do you feel about splitting the error message (argument vs arguments)?

@ivuk Thanks! Looks pretty good -- but I see a few issues.

First, I agree with @spkersten -- it would be better to have a separate messages for 1 and 2+ arguments.

Another thing that actually makes this a bit tricky: Python has some built-in functions that don't accept keyword arguments, such as ord(). Here the new message does not make sense. In the example below ord is treated differently from my_ord, because ord is a built-in function (but some built-in functions accept keyword arguments):

>>> ord()
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: ord() takes exactly one argument (0 given)
>>> def my_ord(c): pass
... 
>>> my_ord()
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: my_ord() missing 1 required positional argument: 'c'

However, mypy (at least currently) does not distinguish between ord and my_ord, which is a bit sad. However, we can use a simple heuristic to not give misleading messages, at least most of the time. If the caller does not provide any keyword arguments, we could fall back to the original error message, such as Too few arguments for "f". If the caller uses at least a single keyword argument, we can use your new, more informative error message. Could you make this change?

With the above refinement the only confusing case is when some code tries to call a built-in function with keyword arguments, even though it shouldn't be valid. I added a new issue to address this issue: #516. You don't need to worry about #516, though.

OK, I hope I got it right. The test cases I used now look like this:

def a(x, y): pass
a(x=1)

def b(x, y): pass
b(y=1)

def c(x, y, z): pass
c(x=1)

def d(x, y, z): pass
d(y=1)

def e(x, y, z): pass
e(z=1)

def f(x): pass
f()

def g(x, y): pass
g()

def h(x, y, z): pass
h()

And the output is this:

$ mypy test.py 
test.py, line 4: Missing positional argument "y" in call to "a"
test.py, line 7: Missing positional argument "x" in call to "b"
test.py, line 10: Missing positional arguments "y", "z" in call to "c"
test.py, line 13: Missing positional arguments "x", "z" in call to "d"
test.py, line 16: Missing positional arguments "x", "y" in call to "e"
test.py, line 19: Too few arguments for "f"
test.py, line 22: Too few arguments for "g"
test.py, line 25: Too few arguments for "h"

Looks good!

@JukkaL, great! :) What's the next step, should I update the failing test cases?

Yes, you should update the failing test cases and potentially add some new test cases if they don't cover all the interesting cases.

You can try running tests.py -i or tests.py -u -- the prior updates test cases in-place to match the current output, and the latter will create new .test.new files with updates from the current output. Sometimes just running tests.py -i is all you need to fix failing test cases.

One more point: if you use the automatic test case update feature, review the changes and make sure they are okay before committing. I'm not sure how reliable it is :)

It's just a handful of them, so no worries there. :)

I've came across a test I'm unsure about, it's in check-varargs.test:

[case testCallingWithTupleVarArgs]
from typing import Undefined
a = Undefined # type: A
b = Undefined # type: B
c = Undefined # type: C
cc = Undefined # type: CC

f(*(a, b, b)) # E: Argument 1 to "f" has incompatible type "Tuple[A, B, B]"; expected "A"
f(*(b, b, c)) # E: Argument 1 to "f" has incompatible type "Tuple[B, B, C]"; expected "A"
f(a, *(b, b)) # E: Argument 2 to "f" has incompatible type "Tuple[B, B]"; expected "B"
f(b, *(b, c)) # E: Argument 1 to "f" has incompatible type "B"; expected "A"
f(*(a, b))    # E: Too few arguments for "f"
f(*(a, b, c, c)) # E: Too many arguments for "f"
f(a, *(b, c, c)) # E: Too many arguments for "f"
f(*(a, b, c))
f(a, *(b, c))
f(a, b, *(c,))
f(a, *(b, cc))

def f(a: 'A', b: 'B', c: 'C') -> None: pass

class A: pass
class B: pass
class C: pass
class CC(C): pass
[builtins fixtures/tuple.py]

With the new syntax, the output for 'Too few arguments for "f"' ends up as:

Alignment of first line difference:
  E: main, line 11: Too few arguments for "f"
  A: main, line 11: Missing positional arguments "a", "b", "c" in call to "f"

I'm not really sure that's correct in this situation, so I figured I'd get a second opinion. :)

How do you feel about this? From what I can tell (by using the inspect module), arg_names gets set to [None] in this case, hence the output mentioned above.

Might be an existing bug. The Argument 1 to "f" has incompatible type "Tuple[A, B, B]"; expected “A” message is not accurate either.

Yeah, looks like an existing bug. It's okay to have the new output and give argument names for now in this special case. Maybe you could you file an issue for this so that it can be sorted out later, and also mention the Tuple[...] issue that Sander pointed out.

Running python3 tests.py seems to succeed for me, both on Ubuntu 12.04 (Python 3.2.3) and Ubuntu 14.10 (Python 3.4.2), I'm not really sure what I need to do to get Travis to play nice. @spkersten, @JukkaL, any tips?

I think that is failing when is performed the static analysis to the mypy code.

Actually, in mypy/messages.py, lines 407 and 408, when you try to get the attribute "arg_names" from Context type, that don't have it.

@rockneurotiko is right. context has the static type Context, which doesn't have a field called arg_names. Currently, context always happens to be of dynamic type CallExpr, which has such a field. So when you run tests (or mypy in general) everything work, but when mypy is run on it self it finds this type error.

You could solve this by giving too_few_arguments an additional argument called something like (beter than) names. From check_argument_count, you can then pass actual_names to too_few_arguments. (I did't try or test this.)

You might also consider renaming the too_few_arguments function, because it has become more of a 'missing_keyword_argument' function than a 'too few arguments' function.

Btw, I noticed that the invalid_argument_count function is never called. I'm not sure if that is related to anything you did, but you could consider cleaning it up (i.e. removing it.)

@spkersten thanks for the taking the time to explain this, I really appreciate it!

I did a quick hack to try out your proposition with passing in actual_names, and it seems to be working properly. This change makes a single test fail (testWithStmtAndInvalidEnter), but I figure I'll work around that one by changing the if a bit (if argument_names is not None and len(argument_names) >= 1).

@JukkaL, if you agree, I'd like to implement this the way @spkersten suggested, it's not complicated.

As far as the invalid_argument_count is considered, I didn't modify that one at all, but a quick grep through the source code shows that it's not used at all, AFAICS. @JukkaL, should it be removed? I'm asking because once we solve the issue that currently makes the tests fail, mypy will complain about the wrong number of arguments that gets passed from invalid_argument_count. :)

730b4ae implements what @spkersten suggested, now the tests are failing because of the invalid_argument_count. I've tried commenting it out, both mypy tests.py and python3 tests.py seem to run properly after that.

I've removed the invalid_argument_count function, since it looks like it's not used at all, the tests are now passing. @spkersten, thanks once more for the effort you put into explaining this, it's much appreciated. :)

@JukkaL, is this an acceptable solution? I don't see any downside to it, but I could be missing something.

@ivuk, you're welcome.

Hmm, it looks mostly good, but this test case (in mypy/test/data/check-namedtuple.test) does not seem right:

[case testCreateNamedTupleWithPositionalArguments]
from collections import namedtuple
from typing import Undefined
X = namedtuple('X', ['x', 'y'])
x = X(1, 'x')
x.x
x.z      # E: "X" has no attribute "z"
x = X(1) # E: Missing positional arguments "x", "y" in call to "X"
x = X(1, 2, 3)  # E: Too many arguments for "X"

Why does it complain about two missing positional arguments? Too few arguments... would seem consistent.

For this particular test case, argument_names gets set to [None], and callee.arg_names to ['x', 'y'].

The check I'm using, if argument_names is not None and len(argument_names) >= 1:, does not catch that.

If I were to extend the test to something like if argument_names is not None and not all(k is None for k in argument_names) and len(argument_names) >= 1:, would that be an acceptable solution? It would catch this situation, but also "break" a number of test cases (six, to be exact, that would revert to outputting the Too few arguments string). For all those (six) test cases, argument_names gets set to [] or [None], so it seems to me that something like this would properly catch all such situations.

That sounds like a reasonable approach. We may be able to relax the condition and give argument names in more messages once we've addressed #516.

Last few commits implement the aforementioned solution and revert most of the test cases to the "old" output, seems OK to me. :)

Looks good now!